Add support for HTML indexes #719
Conversation
charliermarsh
added
the
enhancement
charliermarsh
force-pushed
the
charlie/html
branch
4 times, most recently
from
fc8acef to
f3afff9
Compare
charliermarsh
force-pushed
the
charlie/html
branch
from
f3afff9 to
49240e7
Compare
|
Why the rush? :p |
|
I didn’t assume anyone would be reviewing anything for the next week and I don’t want PRs to sit, so erring on the side of merging. If anyone wants to review they are welcome and I will fix in follow-up commits. |
| #[error("Invalid `Content-Type` header for {0}")] | ||
| InvalidContentTypeHeader(Url, #[source] http::header::ToStrError), | ||
|
|
||
| #[error("Unsupported `Content-Type` \"{1}\" for {0}")] |
There was a problem hiding this comment.
This should tell the user which content types we support
| Utf8(#[from] std::str::Utf8Error), | ||
|
|
||
| #[error(transparent)] | ||
| UrlParse(#[from] url::ParseError), |
There was a problem hiding this comment.
This should also show the input string, url::ParseError is a value-less enum
|
|
||
| /// Parse the list of [`File`]s from the simple HTML page returned by the given URL. | ||
| pub(crate) fn parse_simple(text: &str, base: &Url) -> Result<Vec<File>, Error> { | ||
| let dom = tl::parse(text, tl::ParserOptions::default()).unwrap(); |
There was a problem hiding this comment.
This needs an unwrap-safety comment
| // Extract the hash, which should be in the fragment. | ||
| let hashes = url | ||
| .fragment() | ||
| .map(|fragment| parse_hash(fragment, &url)) | ||
| .transpose()? | ||
| .ok_or_else(|| Error::MissingHash(url.clone()))?; |
There was a problem hiding this comment.
I don't think we can make hashes mandatory, in PEP 503 hashes are a SHOULD.
There was a problem hiding this comment.
Agreed, but they’re currently required everywhere else.
| /// Return the `Accept` header value for all supported media types. | ||
| #[inline] | ||
| const fn accepts() -> &'static str { | ||
| "application/vnd.pypi.simple.v1+json, application/vnd.pypi.simple.v1+html;q=0.2, text/html" |
There was a problem hiding this comment.
Added in the relative URLs PR but I’ll carve it out into a separate change if that doesn’t merge soon.
|
Thanks for the nice review! |
Summary
This PR adds support for HTML index responses (as with
--index-url=https://download.pytorch.org/whl).Closes #412.